Fix migration race condition in concurrent container startups#19
Merged
techbuzzz merged 4 commits intoJan 27, 2026
Merged
Conversation
…LICT Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com>
Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com>
Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] WIP Address feedback on release v0.3.5 PR
Fix migration race condition in concurrent container startups
Jan 27, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a race condition in the database migration system where multiple application instances starting simultaneously could cause PRIMARY KEY conflicts when recording completed migrations. The solution implements atomic migration claiming using PostgreSQL's INSERT ... ON CONFLICT DO NOTHING RETURNING pattern, ensuring only one instance executes each migration while others skip gracefully.
Changes:
- Atomic migration claiming mechanism that prevents concurrent execution conflicts
- Modified error handling to distinguish expected concurrent skips (
sql.ErrNoRows) from actual database errors - Removed transaction wrapping from migrations (tracking rows now inserted before DDL execution)
- Added integration test simulating 5 concurrent migration attempts
- Created comprehensive documentation explaining the concurrency safety approach and trade-offs
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| docs/MIGRATION_CONCURRENCY.md | New documentation file explaining the race condition problem, solution approach with code examples, benefits, trade-offs, and testing instructions |
| cmd/server/migrations_test.go | New integration test that verifies concurrent migration safety by running 5 goroutines simultaneously and checking for single execution |
| cmd/server/main.go | Core migration logic updated to use INSERT ON CONFLICT for atomic claiming, removed transactions, added cleanup logic for file read failures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+302
to
+305
| `INSERT INTO schema_migrations (version, applied_at) | ||
| VALUES ($1, CURRENT_TIMESTAMP) | ||
| ON CONFLICT (version) DO NOTHING | ||
| RETURNING version`, |
There was a problem hiding this comment.
Inconsistent indentation: the SQL query uses tabs for the first level of indentation (VALUES, ON CONFLICT, RETURNING) but the surrounding Go code likely uses tabs consistently. The query string should use consistent indentation, preferably aligning with common Go formatting conventions for multi-line strings.
Suggested change
| `INSERT INTO schema_migrations (version, applied_at) | |
| VALUES ($1, CURRENT_TIMESTAMP) | |
| ON CONFLICT (version) DO NOTHING | |
| RETURNING version`, | |
| `INSERT INTO schema_migrations (version, applied_at) | |
| VALUES ($1, CURRENT_TIMESTAMP) | |
| ON CONFLICT (version) DO NOTHING | |
| RETURNING version`, |
Comment on lines
+51
to
+68
| for i := 0; i < numConcurrent; i++ { | ||
| wg.Add(1) | ||
| go func(id int) { | ||
| defer wg.Done() | ||
|
|
||
| // Each goroutine creates its own DB connection | ||
| db, err := database.NewDB(dbURL) | ||
| if err != nil { | ||
| errors <- err | ||
| return | ||
| } | ||
| defer db.Close() | ||
|
|
||
| // Attempt to run migrations | ||
| if err := runMigrations(db); err != nil { | ||
| errors <- err | ||
| } | ||
| }(i) |
There was a problem hiding this comment.
The test creates database connections inside each goroutine but doesn't ensure all goroutines start executing runMigrations simultaneously, which could reduce the effectiveness of testing the race condition. Consider using a sync mechanism like sync.WaitGroup with a countdown and wg.Wait() to ensure all goroutines reach the migration execution point before any proceed, maximizing the chance of detecting race conditions. For example, add a shared startWg that each goroutine adds to, then all wait on before calling runMigrations.
Comment on lines
+30
to
+33
| if err != nil { | ||
| // sql.ErrNoRows means ON CONFLICT happened - another instance claimed this | ||
| log.Printf("Migration %s already claimed by another instance, skipping", entry.Name()) | ||
| continue |
There was a problem hiding this comment.
The error handling shown in this documentation example is inconsistent with the actual implementation in main.go. The actual implementation at lines 309-317 checks for sql.ErrNoRows first and then handles other errors separately with an else if. This documentation suggests any error triggers the skip behavior, which is incorrect. The documentation should show the same two-branch error handling pattern used in the actual code.
Suggested change
| if err != nil { | |
| // sql.ErrNoRows means ON CONFLICT happened - another instance claimed this | |
| log.Printf("Migration %s already claimed by another instance, skipping", entry.Name()) | |
| continue | |
| if errors.Is(err, sql.ErrNoRows) { | |
| // ON CONFLICT happened - another instance claimed this | |
| log.Printf("Migration %s already claimed by another instance, skipping", entry.Name()) | |
| continue | |
| } else if err != nil { | |
| // Any other error means something went wrong claiming the migration | |
| log.Printf("Failed to claim migration %s: %v", entry.Name(), err) | |
| return err |
Comment on lines
+26
to
+32
| // Create temporary test migration file | ||
| tmpDir := t.TempDir() | ||
| migrationFile := filepath.Join(tmpDir, "001_test.sql") | ||
| migrationSQL := `CREATE TABLE IF NOT EXISTS test_table (id SERIAL PRIMARY KEY, name TEXT);` | ||
| if err := os.WriteFile(migrationFile, []byte(migrationSQL), 0644); err != nil { | ||
| t.Fatalf("Failed to create test migration: %v", err) | ||
| } |
There was a problem hiding this comment.
The test creates a migration file at line 28 in tmpDir, then creates a migrations subdirectory at lines 36-42 with the same migration. The file created at line 28 is never used and could cause confusion. Only the migration in the migrations/ subdirectory is needed for the test since runMigrations reads from that directory.
Suggested change
| // Create temporary test migration file | |
| tmpDir := t.TempDir() | |
| migrationFile := filepath.Join(tmpDir, "001_test.sql") | |
| migrationSQL := `CREATE TABLE IF NOT EXISTS test_table (id SERIAL PRIMARY KEY, name TEXT);` | |
| if err := os.WriteFile(migrationFile, []byte(migrationSQL), 0644); err != nil { | |
| t.Fatalf("Failed to create test migration: %v", err) | |
| } | |
| // Set up temporary migrations directory and test migration file | |
| tmpDir := t.TempDir() | |
| migrationSQL := `CREATE TABLE IF NOT EXISTS test_table (id SERIAL PRIMARY KEY, name TEXT);` |
Comment on lines
+302
to
+303
| `INSERT INTO schema_migrations (version, applied_at) | ||
| VALUES ($1, CURRENT_TIMESTAMP) |
There was a problem hiding this comment.
The SQL INSERT statement uses applied_at in the VALUES clause but also sets it via CURRENT_TIMESTAMP. However, the schema at line 256 shows that applied_at has a DEFAULT CURRENT_TIMESTAMP clause. Explicitly specifying CURRENT_TIMESTAMP in the INSERT is redundant with the default and could be omitted for cleaner code, or the column name should be included in the INSERT if you want to be explicit about all columns being set.
Suggested change
| `INSERT INTO schema_migrations (version, applied_at) | |
| VALUES ($1, CURRENT_TIMESTAMP) | |
| `INSERT INTO schema_migrations (version) | |
| VALUES ($1) |
Comment on lines
+332
to
+333
| // Note: We don't use a transaction here because we've already inserted the tracking row | ||
| // If the migration fails, the tracking row remains as a record that it was attempted |
There was a problem hiding this comment.
The comment says 'We don't use a transaction here because we've already inserted the tracking row', but it doesn't explain why this is the desired behavior. Consider expanding this comment to mention the trade-off: without a transaction, if DDL execution fails, the tracking row remains, which prevents automatic retries and requires manual intervention. This is intentional to avoid retry loops with failing migrations, as documented in MIGRATION_CONCURRENCY.md.
Suggested change
| // Note: We don't use a transaction here because we've already inserted the tracking row | |
| // If the migration fails, the tracking row remains as a record that it was attempted | |
| // Note: We intentionally do not wrap this in a transaction because the tracking row | |
| // in schema_migrations has already been inserted to "claim" the migration. If the DDL | |
| // execution fails, the tracking row is left in place. This prevents automatic retries | |
| // of a migration that is known to be failing, avoiding retry loops across multiple | |
| // instances. Instead, manual intervention is required to fix the migration and adjust | |
| // the tracking row if appropriate. See MIGRATION_CONCURRENCY.md for the full rationale. |
Comment on lines
+43
to
+44
| defer os.Chdir(oldWd) | ||
| os.Chdir(tmpDir) |
There was a problem hiding this comment.
The test changes the working directory at line 44 but this could affect other tests running concurrently in the same process. While t.TempDir() provides isolation for the temporary directory, changing the process-wide current working directory with os.Chdir() is not test-safe. Consider using the full path to the migrations directory instead of relying on a relative path, or ensure the runMigrations function accepts a directory parameter for testing.
| } | ||
|
|
||
| // Change to temp directory for migration discovery | ||
| oldWd, _ := os.Getwd() |
There was a problem hiding this comment.
The error from os.Getwd() is ignored using the blank identifier. If os.Getwd() fails, oldWd will be an empty string, and the deferred os.Chdir(oldWd) will fail silently, leaving the working directory changed. This error should be checked and the test should fail if it cannot obtain the current working directory.
Comment on lines
+88
to
+94
| err = db.QueryRow("SELECT COUNT(*) FROM schema_migrations WHERE version = '001_test.sql'").Scan(&count) | ||
| if err != nil { | ||
| t.Fatalf("Failed to query migrations: %v", err) | ||
| } | ||
| if count != 1 { | ||
| t.Errorf("Expected migration to be recorded exactly once, got %d times", count) | ||
| } |
There was a problem hiding this comment.
The test checks that exactly one migration record exists in schema_migrations (line 92), but the test doesn't verify that the migration was executed only once. If the migration SQL CREATE TABLE IF NOT EXISTS test_table runs multiple times, it would succeed each time due to IF NOT EXISTS. A more robust test would use a migration that fails on duplicate execution (e.g., CREATE TABLE test_table without IF NOT EXISTS) to verify the claiming mechanism truly prevents concurrent execution, or track execution count in a different way.
Comment on lines
+300
to
+307
| var claimedVersion string | ||
| err = db.QueryRow( | ||
| `INSERT INTO schema_migrations (version, applied_at) | ||
| VALUES ($1, CURRENT_TIMESTAMP) | ||
| ON CONFLICT (version) DO NOTHING | ||
| RETURNING version`, | ||
| entry.Name(), | ||
| ).Scan(&claimedVersion) |
There was a problem hiding this comment.
The variable claimedVersion is declared and populated via Scan() but never used afterward. While this is necessary for the Scan() call to work (scanning the RETURNING value distinguishes successful INSERT from ON CONFLICT), consider adding a comment explaining why this variable exists, or adding a sanity check like if claimedVersion != entry.Name() to verify the claim succeeded for the expected migration.
techbuzzz
added a commit
that referenced
this pull request
Jan 28, 2026
* chore(gitignore): 🗃️ add `postgres_data/` to .gitignore * Ensures that the `postgres_data/` directory and its contents are excluded from version control. * Prevents accidental commits of local PostgreSQL data files. * feat(tasks): add task reassignment functionality with new API endpoint and UI modal * feat(mcp): enhance MCPHandler to broadcast agent updates via WebSocket feat(TaskCard): display task ID in the TaskCard component * fix(TaskCard): adjust layout to properly display task ID and due date * fix(TaskCard): remove due date display from task card * Release v0.3.5 – Daily Standup Management & Production Migrations (#10) * feat: add daily standup and agent heartbeat models, migrations, and Vue components - Implemented DailyStandup and AgentHeartbeat models in Go with necessary fields. - Created SQL migrations for daily_standups and agent_heartbeats tables with indexes. - Developed StandupModal component for submitting and editing daily standups with form validation. - Added standupStore for managing standup data, including fetching, creating, updating, and deleting standups. - Created Standups view to display a list of daily standups with filtering options and modal integration. - Enhanced UI with loading states, error handling, and markdown rendering for standup details. * fix(go.mod): remove 'indirect' comments from dependency requirements * feat(migrations): implement automatic migration system with tracking and bootstrap scripts * feat: add release notes for v0.3.5 including daily standup management and migration system * Initial plan * Initial plan * Initial plan * Add application verification report for v0.3.5 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Update internal/handlers/standups.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update scripts/bootstrap-migrations.ps1 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Initial plan (#20) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Refactor task reassignment to use shared API client (#18) * Initial plan * Refactor task reassignment to use shared API client Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Fix migration race condition in concurrent container startups (#19) * Initial plan * Make migration loop safe for concurrent startups using INSERT ON CONFLICT Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Add concurrency safety test and documentation for migrations Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Address code review feedback: improve error handling and add sql import Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Filter migrations to only run numbered files, skip bootstrap helpers (#17) * Initial plan * feat(migrations): filter to only run numbered migrations, skip helper scripts Add regex pattern to only execute numbered migrations (e.g., 001_init.sql, 002_sample_data.sql) and skip helper scripts like bootstrap_existing_db.sql. This prevents the bootstrap script from being run automatically during the migration process, maintaining the intended "run once for existing DBs" bootstrap flow. Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * refactor: move regex compilation outside loop for efficiency Move migration pattern compilation to the beginning of runMigrations function to avoid repeated compilation overhead on each function call. Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Add aria-label to icon-only buttons in Standups view (#16) * Initial plan * feat(a11y): add aria-label to edit/delete buttons in Standups view Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Fix async state management in ReassignModal (#15) * Initial plan * Fix isSubmitting state management in ReassignModal - Move isSubmitting state to parent (ProjectDetail) - Pass isSubmitting as prop to ReassignModal - Properly manage async state throughout reassignment lifecycle - Button stays disabled until API request completes Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * fix(standups): return 404 on non-existent update and broadcast changes (#14) * Initial plan * fix(standups): improve UpdateStandup to check rows affected and return updated object Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * fix(standups): handle error from RowsAffected in UpdateStandup Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Fix metadata loss in AgentCard legacy capability unmarshaling (#13) * Initial plan * fix: preserve legacy capabilities in metadata after struct assignment Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * style: remove extra blank line for consistency Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Address PR #11 review comments: preserve metadata, add 404 checks, improve concurrency safety and accessibility (#12) * Initial plan * fix: preserve legacy capabilities in metadata and add 404 checks for standup updates/deletes Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * fix(ui): improve accessibility, state management, and date handling in frontend components Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * fix(migrations): skip bootstrap files and add advisory lock for concurrency safety Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * fix(api): use API service for task reassignment and sanitize database URL in logs Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * refactor: improve code quality based on review feedback Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> Co-authored-by: Victor Buzin <buzin.victor@gmail.com> * Update web/src/services/api.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update internal/handlers/standups.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Initial plan (#21) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Initial plan (#22) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Initial plan (#23) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Restore transactional migration execution with automatic rollback (#24) * Initial plan * Restore transactional migration execution with rollback safety Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Improve transaction handling with defer cleanup and race condition handling Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Extract migration application into separate function for proper defer scoping Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Add missing database/sql import (#25) * Initial plan * Add missing database/sql import to fix compile error Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Make database migrations transactional (#26) * Initial plan * refactor: make migrations transactional and safe - Execute migration DDL within a transaction - Record migration in schema_migrations only after DDL succeeds - Commit both DDL and tracking record atomically - Proper rollback on any failure - Now matches "Automatic, transactional, safe" claim in verification report Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * refactor: simplify migration transaction logic per code review - Remove complex ON CONFLICT detection with RETURNING - Use simple ON CONFLICT DO NOTHING for defense-in-depth - Remove unused database/sql import - Improve comments explaining PostgreSQL transactional DDL support - Simplify error handling paths Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> Co-authored-by: Victor Buzin <buzin.victor@gmail.com> * Sort migration files to ensure deterministic execution order (#27) * Initial plan * Sort migration files to ensure deterministic execution order Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Fix readonly prop mutation in ReassignModal (#28) * Initial plan * Fix isSubmitting prop mutation in ReassignModal Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Fix isReassigningTask state not resetting after reassignment (#29) * Initial plan * Fix isReassigningTask state management by adding finally block Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> Co-authored-by: Victor Buzin <buzin.victor@gmail.com> * Fix migration race: execute DDL before recording completion (#30) * Initial plan * Fix migration safety: execute DDL before recording success Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Remove server binary and add to gitignore Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Address code review: check rollback errors and simplify ON CONFLICT Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> Co-authored-by: Victor Buzin <buzin.victor@gmail.com> * Enhance sample data migration: add idempotent comments and ensure conflict handling for inserts * Refactor standup model and handlers: rename references to reference_links for consistency * Add MCP Setup Documentation and Verification Files - Created MCP_SETUP_QUICK_REFERENCE.md for quick setup and command usage. - Added MCP_SETUP_REVIEW.md detailing component verification and functionality. - Introduced MCP_SETUP_VERIFICATION_SUMMARY.md summarizing the verification process and results. * Add comprehensive documentation for Visual Studio 2026 MCP integration - Created VS2026_DOCUMENTATION_INDEX.md for quick navigation and role-based reading guide. - Added VS2026_FEATURE_SUMMARY.md detailing key features, deployment options, and generated files. - Introduced VS2026_IMPLEMENTATION_GUIDE.md with step-by-step setup instructions and UI integration examples. - Developed VS2026_MCP_SETUP.md outlining configuration files, usage examples, and troubleshooting tips. - Enhanced composable with new functions and improved error handling. * Refactor MCP setup: enhance error handling, add support for Blob downloads, and update event handlers for better user feedback * Refactor code structure for improved readability and maintainability * [WIP] WIP Address feedback on Release v0.3.5 changes (#31) * Initial plan * Fix mcpApiUrl to use hostname instead of host to avoid double port issue Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Remove unused isRef import from useMcpSetup composable (#32) * Initial plan * Remove unused isRef import from useMcpSetup.js Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Update cmd/server/main.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update internal/handlers/standups.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix advisory lock session scope in database migrations (#33) * Initial plan * Fix advisory lock to use dedicated connection for session scope Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Remove UTF-8 BOM from migration file (#34) * Initial plan * Remove UTF-8 BOM from migrations/003_daily_standups.sql Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Fix backward compatibility: restore `references` JSON field for standup models (#35) * Initial plan * Fix backward compatibility: change JSON tag from reference_links to references Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Harden formatDate validation to prevent RangeError on invalid inputs (#36) * Initial plan * Add validation for formatDate to prevent RangeError with invalid dates Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Improve date validation: add rollover check and error handling Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Refactor formatDate: extract fallback parsing to eliminate duplication Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Remove shadowing imports in ProjectDetail.vue (#37) * Initial plan * Remove unused downloadFile and downloadAllMcpFiles imports Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Fix .mcp download to use VS2026 config instead of VSCode config (#38) * Initial plan * Fix .mcp download to use VS2026 config instead of VSCode config Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> * Align VS2026 MCP documentation with minimal config implementation (#39) * Initial plan * Update VS2026 docs to match actual minimal config implementation Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: techbuzzz <6938100+techbuzzz@users.noreply.github.com>
techbuzzz
deleted the
copilot/sub-pr-11-b57e510c-7092-4196-8d68-518b5ca0c288
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The migration loop had a race condition where concurrent instances could both execute the same migration DDL, then one would crash on PRIMARY KEY conflict when inserting the tracking row.
Changes
Atomic migration claiming: Use
INSERT ... ON CONFLICT DO NOTHING RETURNING versionto claim migrations before executionsql.ErrNoRowsand skip gracefullyError handling: Distinguish
sql.ErrNoRows(expected concurrent skip) from actual database errorsCleanup on failure: Remove tracking row if migration file unreadable after claim
Trade-off: Failed migrations leave tracking rows requiring manual cleanup (intentional—prevents retry loops).
Testing
Added integration test simulating 5 concurrent migration attempts. Verified single execution and no conflicts.
See
docs/MIGRATION_CONCURRENCY.mdfor detailed explanation.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.